Skip to content

fix(cdc): stop arming OUT endpoint with a NULL buffer - #3058

Open
KentLee86 wants to merge 2 commits into
stm32duino:mainfrom
KentLee86:fix-cdc-null-rx-buffer
Open

fix(cdc): stop arming OUT endpoint with a NULL buffer#3058
KentLee86 wants to merge 2 commits into
stm32duino:mainfrom
KentLee86:fix-cdc-null-rx-buffer

Conversation

@KentLee86

@KentLee86 KentLee86 commented Aug 25, 2026

Copy link
Copy Markdown

Summary

USBD_CDC_Receive() re-arms the bulk OUT endpoint with a NULL application
buffer when the CDC receive queue is full. The next OUT packet is then copied
to address 0 by the PCD interrupt handler and the MCU faults. This PR stops
arming the endpoint in that case and lets it NAK instead.

This PR fixes/implements the following bugs/features

Motivation

USBD_CDC_Receive() commits the received block and then tries to reserve the
next one:

https://github.com/stm32duino/Arduino_Core_STM32/blob/4a8b28a08a2c4ba2e1a76e15a1f0d6de1e39ff7f/libraries/USBDevice/src/cdc/usbd_cdc_if.c#L241-L247

CDC_resume_receive() returns false when CDC_ReceiveQueue_ReserveBlock()
cannot reserve another 64 byte slot, and the fallback USBD_CDC_ClearBuffer()
arms the endpoint with a null buffer and a null length:

https://github.com/stm32duino/Arduino_Core_STM32/blob/4a8b28a08a2c4ba2e1a76e15a1f0d6de1e39ff7f/libraries/USBDevice/src/cdc/usbd_cdc.c#L1022-L1043

The endpoint is valid again while ep->xfer_buff is NULL. When the host
sends the next packet, the "OUT Single Buffering" branch of the PCD interrupt
handler copies it without checking the destination — only count is checked:

https://github.com/stm32duino/Arduino_Core_STM32/blob/4a8b28a08a2c4ba2e1a76e15a1f0d6de1e39ff7f/system/Drivers/STM32F1xx_HAL_Driver/Src/stm32f1xx_hal_pcd.c#L2307-L2317

(The EP0 branch a few lines above does guard it, with
(ep->xfer_count != 0U) && (ep->xfer_buff != 0U).)

So the packet is written to address 0.

An unarmed bulk OUT endpoint NAKs and the host retries, which is the flow
control this situation calls for. USBSerial::read(), readBytes(),
readBytesUntil() and readStringUntil() all call CDC_resume_receive()
after dequeuing, so the endpoint is armed again with a real block as soon as
the sketch drains anything. Nothing else needs to change.

The second commit removes USBD_CDC_ClearBuffer(), which had no other caller.
It is local to this core (the vendored ST middleware copy in
system/Middlewares/ST/STM32_USB_Device_Library/Class/CDC/Src/usbd_cdc.c has
no such function), and arming an endpoint with a null buffer is not something
that is safe to call. Its USE_USBD_COMPOSITE branch also would not compile —
it reads a bare classId instead of pdev->classId. Happy to drop that
commit if you would rather keep the function around.

Validation

Board: STM32F103C8 "Blue Pill", USB CDC on PA11/PA12.

Reproducer — nothing but a drain loop:

#include <Arduino.h>

void setup() {
  SerialUSB.begin();
}

void loop() {
  while (SerialUSB.available() > 0) {
    (void)SerialUSB.read();
  }
  static uint32_t last = 0;
  if (millis() - last >= 1000) {
    last = millis();
    SerialUSB.println("alive");
  }
}

Open the CDC port on the host and do a single write(b"a" * 512). The board
stops printing alive, stops answering on USART1 as well, and stays dead
until reset. The same 512 bytes written in 16 byte chunks 20 ms apart are
handled fine, because the sketch drains in between — it is bytes in flight,
not length or content.

Halting the core over SWD right after the crash:

IPSR  = 3 (HardFault),  PC = Default_Handler (the b.n self-loop)
CFSR  = 0x00000400  -> BFSR bit 2, IMPRECISERR (buffered write)
HFSR  = 0x40000000  -> FORCED
Exception frame: R0 = 0x40006140 (USB PMA)   R1 = 0x00000000 (destination)
                 R2 = 0x61 ('a', the test byte)   R3 = 0x40 (64, packet size)
Stacked PC -> USB_ReadPMA,  stacked LR -> HAL_PCD_IRQHandler

Measured on core 2.10.1 (PlatformIO framework-arduinoststm32 4.21001.250617).
stm32f1xx_hal_pcd.c is byte identical between 2.10.1 and current main, and
the two CDC hunks above are unchanged too, so main is affected the same way.

The runtime behaviour of this patch was verified on that hardware by making
the USBD_CDC_ClearBuffer() call a no-op with
-Wl,--wrap=USBD_CDC_ClearBuffer, which leaves exactly the code path this PR
leaves. At the stock queue size the sketch above then handles a single 16 KB
host write correctly, as does the larger application the bug was found in.
I have not re-run the hardware test with this source patch built from main.

Raising CDC_RECEIVE_QUEUE_BUFFER_PACKET_NUMBER — the workaround suggested in
#1399 — only moves the ceiling. In the larger application the fault moved from
130 bytes to about 1536 bytes; it does not remove it.

Build check for this patch: PlatformIO bluepill_f103c8, framework = arduino
pointed at this branch, sketch above, builds clean.

Scope

The fix is in libraries/USBDevice, so it applies to every family, but I have
only measured F1.

For what it is worth, the missing xfer_buff check is not specific to F1. In
system/Drivers on main, the "OUT Single Buffering" branch guards only
count != 0U in all 17 PMA based drivers (C0, C5, F0, F1, F3, G0, G4, H5, L0,
L1, L4, L5, U0, U3, U5, WB, WBA), and the OTG RXFLVL path guards only
BCNT != 0U in F2, F4, F7 and H7. That is vendored ST code, so I have not
touched it here — with this PR the core no longer hands those drivers a null
buffer.

Code formatting

Checked locally with CI/astyle/.astylerc (astyle 3.6.18 here, CI pins 3.1):
no reformatting on any of the three files, and none on the unpatched main
versions either, so the astyle version difference is not hiding anything.

Closing issues

Fixes #1399

dldntjr407 and others added 2 commits August 26, 2026 03:09
USBD_CDC_Receive() falls back to USBD_CDC_ClearBuffer() when the
receive queue has no room left for another 64 byte block. That helper
re-arms the OUT endpoint with a NULL application buffer:

    USBD_LL_PrepareReceive(pdev, CDC_OUT_EP, 0, 0);

The endpoint is valid again while ep->xfer_buff is NULL. The next OUT
packet the host sends reaches HAL_PCD_IRQHandler(), which copies the
packet without checking the destination:

    if (count != 0U)
    {
      USB_ReadPMA(hpcd->Instance, ep->xfer_buff, ep->pmaadress, count);
    }

The packet lands at address 0 and the write faults. On a STM32F103C8
this is an imprecise bus fault escalated to a hard fault (CFSR
IMPRECISERR, HFSR FORCED), and the board stops answering until reset.

Do not arm the endpoint when there is no room. An unarmed bulk OUT
endpoint NAKs, the host retries, and USBSerial::read() and the
readBytes() family already call CDC_resume_receive() after dequeuing,
so the endpoint is armed with a real block as soon as the sketch
drains. That is the flow control this transfer needs.

Fixes stm32duino#1399

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
USBD_CDC_Receive() was its only caller. The helper is not part of the
ST USB device middleware, it is local to this core, and the only thing
it does is arm the OUT endpoint with a NULL application buffer, which
is what made a receive overrun fault the MCU.

Remove it so it cannot be reused. This also drops a USE_USBD_COMPOSITE
branch that would not compile: it reads a bare classId instead of
pdev->classId.
Copilot AI lite review requested due to automatic review settings August 25, 2026 18:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents USB CDC bulk OUT endpoint re-arming with a NULL Rx buffer when the CDC receive queue is full, avoiding a hard fault caused by low-level USB drivers copying incoming OUT data to address 0x0. Instead, when the queue is full, the OUT endpoint is left unarmed so it NAKs and the host retries until the sketch drains data and CDC_resume_receive() can arm reception with a valid buffer.

Changes:

  • Remove the USBD_CDC_ClearBuffer() function and its header declarations (it had no remaining callers).
  • Update USBD_CDC_Receive() to always attempt CDC_resume_receive() and otherwise leave the OUT endpoint unarmed (NAK-based flow control), avoiding NULL buffer arming.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
libraries/USBDevice/src/cdc/usbd_cdc.c Removes USBD_CDC_ClearBuffer() so the stack no longer provides an unsafe “arm with NULL buffer” API.
libraries/USBDevice/src/cdc/usbd_cdc_if.c Stops re-arming OUT reception with a NULL buffer on queue-full; relies on NAK + retry until CDC_resume_receive() can arm a real block.
libraries/USBDevice/inc/usbd_cdc.h Removes USBD_CDC_ClearBuffer() declarations to match the implementation removal.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@fpistm

fpistm commented Aug 26, 2026

Copy link
Copy Markdown
Member

Hi @KentLee86
Thanks for the PR. I waill review it but seems in fact you revert this PR #466.
I don't think it is the good way as it probably solve an issue but probably add another one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

STM32F10xx Serial bus fault on large data

4 participants